Skip to content

Trim whitespaces when creating a RepositoryAttribute#437

Merged
Magnus-Kuhn merged 26 commits intomainfrom
whitespace-removal
Mar 6, 2025
Merged

Trim whitespaces when creating a RepositoryAttribute#437
Magnus-Kuhn merged 26 commits intomainfrom
whitespace-removal

Conversation

@Magnus-Kuhn
Copy link
Copy Markdown
Contributor

@Magnus-Kuhn Magnus-Kuhn commented Feb 25, 2025

Readiness checklist

  • I added/updated tests.
  • I ensured that the PR title is good enough for the changelog.
  • I labeled the PR.
  • I self-reviewed the PR.

Notes

Since the CreateAttributeRequestItem has the most sophisticated processing, it also has more detailed tests than the Propose- and ReadAttributeRequestItems. They will need to be adapted once the Propose- and ReadAttributeRequestItemProcessors are updated.

@Magnus-Kuhn Magnus-Kuhn added wip Work in Progress (blocks mergify from auto update the branch) enhancement New feature or request labels Feb 25, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
...ion/src/modules/attributes/AttributesController.ts 91.39% <100.00%> (+0.10%) ⬆️
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Magnus-Kuhn Magnus-Kuhn removed the wip Work in Progress (blocks mergify from auto update the branch) label Feb 28, 2025
@Magnus-Kuhn Magnus-Kuhn marked this pull request as ready for review February 28, 2025 12:07
Copy link
Copy Markdown
Contributor

@britsta britsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the ProposeAttributeRequestItemProcessor, logic for the RepositoryAttribute duplicates will be added in the future and the code for the RepositoryAttribute duplicates in the ReadAttributeRequestItemProcessor (#423) will be adapted/refactored, right? Then I understand the PR description to mean that the trimming of whitespace in these two cases should be implemented in a separate PR after the other changes have been made. 😊

Edit: I saw that the PR description has been updated, so the question has been answered. :D

Copy link
Copy Markdown
Contributor

@britsta britsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last small comment, rest LGTM.

britsta
britsta previously approved these changes Mar 4, 2025
Copy link
Copy Markdown
Contributor

@britsta britsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🐸

@jkoenig134
Copy link
Copy Markdown
Contributor

is this a breaking change? I don't think so, right?

@Magnus-Kuhn
Copy link
Copy Markdown
Contributor Author

is this a breaking change? I don't think so, right?

nope, it's not

@jkoenig134
Copy link
Copy Markdown
Contributor

jkoenig134 commented Mar 6, 2025

@Magnus-Kuhn please take care of the merge conflicts here :)

@Magnus-Kuhn Magnus-Kuhn dismissed stale reviews from Milena-Czierlinski and britsta via e52cb66 March 6, 2025 13:15
jkoenig134
jkoenig134 previously approved these changes Mar 6, 2025
@Magnus-Kuhn Magnus-Kuhn enabled auto-merge (squash) March 6, 2025 13:21
@Magnus-Kuhn Magnus-Kuhn merged commit bba03c6 into main Mar 6, 2025
15 checks passed
@Magnus-Kuhn Magnus-Kuhn deleted the whitespace-removal branch March 6, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants